Refactor createConfigExtensionSpecification to support direct deployConfig and transform params#6908
Refactor createConfigExtensionSpecification to support direct deployConfig and transform params#6908ryancbahan wants to merge 1 commit intomainfrom
Conversation
Coverage report
Test suite run success3781 tests passing in 1449 suites. Report generated by 🧪jest coverage report action from 8b9720e |
|
We detected some changes at Caution DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release. |
ff7c0de to
8b9720e
Compare
| const transformLocalToRemote = | ||
| spec.transformLocalToRemote ?? (spec.transformConfig ? resolveAppConfigTransform(spec.transformConfig) : undefined) | ||
| const transformRemoteToLocal = | ||
| spec.transformRemoteToLocal ?? resolveReverseAppConfigTransform(spec.schema, spec.transformConfig) |
There was a problem hiding this comment.
transformRemoteToLocal is now always set even when transformConfig is omitted
createConfigExtensionSpecification now computes const transformRemoteToLocal = spec.transformRemoteToLocal ?? resolveReverseAppConfigTransform(spec.schema, spec.transformConfig) and resolveReverseAppConfigTransform returns a default reverse-transform when transformConfig is undefined. This means every config extension spec will have a reverse transform unless it explicitly sets transformRemoteToLocal: undefined (not really expressible) or bypasses the factory. Previously, transformConfig was required, so the behavior was explicit.
Why this is risky: callers relying on “no reverse transform” semantics may see keys dropped, content nested according to schema, or input mutation (see next comment), leading to confusing diffs or broken local config; and potentially omitting server fields if the transformed output is re-sent.
Scale: affects any configuration extension spec created without transformConfig (now allowed by types).
|
🤖 Code Review · #projects-dev-ai for questions ✅ Complete - 1 findings 📋 History✅ 1 findings |
| transformConfig?: TransformationConfig | CustomTransformationConfig | ||
| deployConfig?: ExtensionSpecification<TConfiguration>['deployConfig'] | ||
| transformLocalToRemote?: ExtensionSpecification<TConfiguration>['transformLocalToRemote'] | ||
| transformRemoteToLocal?: ExtensionSpecification<TConfiguration>['transformRemoteToLocal'] |
There was a problem hiding this comment.
How would we feel about making transformRemoteToLocal explicit? It's an override, so the default case should be not to implement it at all -- at which point you get the default behavior of using the contract for reverse transform.

Summary
Refactors
createConfigExtensionSpecificationto accept optionaldeployConfig,transformLocalToRemote, andtransformRemoteToLocalparameters directly, alongside the existingtransformConfig. This is a prerequisite for the app module contracts migration — it unblocks all per-module PRs (#6909–#6915).Why this is necessary
Today,
createConfigExtensionSpecificationderives both forward (transformLocalToRemote) and reverse (transformRemoteToLocal) transforms from a singletransformConfigparameter. There is no way to express "no forward transform" —resolveAppConfigTransformalways returns a function.The contract migration requires exactly this: each core app config module needs to stop sending CLI-transformed payloads and instead send TOML-shaped data directly via
deployConfig, while preserving the reverse transform (since the server still returns Layer 2 field names andapp config link/pulldepends ontransformRemoteToLocal).Without this change, per-module migrations would have to either:
transformConfigwith dummy forward functions that are never calledcreateContractBasedModuleSpecification, which removes the reverse transform entirely and breaksapp config linkandapp config pullWhat changes
createConfigExtensionSpecificationnow accepts three new optional parameters:deployConfig— passed through to the extension spec. When set, the deploy path uses this instead oftransformLocalToRemote(the existing fallback chain inextension-instance.ts:211isdeployConfig ?? transformLocalToRemote ?? undefined).transformLocalToRemote— overrides the forward transform derived fromtransformConfig. Can be leftundefinedto express "no forward transform."transformRemoteToLocal— overrides the reverse transform derived fromtransformConfig.Resolution logic:
Backward compatibility
transformConfigis now optional but fully supported — all existing specs using it are unchangedtransformConfigis provided, behavior is identical to beforetransformConfig-derived values when both are providedHow per-module PRs (#6909–#6915) use this
Each module migrates from:
To:
This sends TOML-shaped data directly (via
deployConfig), with no forward transform. The server's write adapter normalizes to Layer 2 for storage. The reverse transform is preserved forapp config link/pull.Test plan
transformConfigusage still derives both transforms correctlydeployConfig+transformRemoteToLocalwork withouttransformConfig(forward transform isundefined)transformConfig-derived values🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com